fix(trustify): merge remediation data across purl_statuses with advisory attribution (TC-5019)#640
Merged
Merged
Conversation
Reviewer's GuideThis PR fixes Trustify issue merging logic so multiple affected/advisory entries for the same CVE+source are consolidated into a single Issue, with deduped fixed versions, multiple remediation entries with advisory attribution, and correct CVSS selection, using new API model types from trustify-da-api 2.0.11-SNAPSHOT. Sequence diagram for merging multiple affected entries into a single IssuesequenceDiagram
participant TrustifyResponseHandler
participant Issue
participant Remediation
participant RemediationInfo
TrustifyResponseHandler->>TrustifyResponseHandler: toIssues(response)
TrustifyResponseHandler->>TrustifyResponseHandler: mergeIssueData(existingIssue, data)
TrustifyResponseHandler->>TrustifyResponseHandler: getAdvisoryScore(existingIssue.id, scores)
TrustifyResponseHandler->>Issue: cvssScore(score.score())
TrustifyResponseHandler->>Issue: setSeverity(severity)
TrustifyResponseHandler->>TrustifyResponseHandler: ensureRemediation(existingIssue)
TrustifyResponseHandler->>Remediation: addFixedVersions(remediation, ranges)
TrustifyResponseHandler->>TrustifyResponseHandler: buildRemediationInfo(data, hasFixedVersions)
TrustifyResponseHandler->>Remediation: addRemediationsItem(remediationInfo)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The CVSS handling in
mergeIssueDataduplicates logic fromsetCvssData; consider extracting a shared helper for choosing and applying the highest advisory score to reduce duplication and keep behavior aligned. - In
buildRemediationInfo, the URL detection usingidentifier.startsWith("http")is quite loose; if possible, tighten this to a more robust URI validation or explicit scheme check to avoid misclassifying non-URL identifiers. - The RemediationCategory assignment in
buildRemediationInfois driven solely byhasFixedVersions; if other remediation types are expected (e.g., configuration changes or workarounds), consider extending this logic to distinguish categories based on advisory content rather than just fixed versions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CVSS handling in `mergeIssueData` duplicates logic from `setCvssData`; consider extracting a shared helper for choosing and applying the highest advisory score to reduce duplication and keep behavior aligned.
- In `buildRemediationInfo`, the URL detection using `identifier.startsWith("http")` is quite loose; if possible, tighten this to a more robust URI validation or explicit scheme check to avoid misclassifying non-URL identifiers.
- The RemediationCategory assignment in `buildRemediationInfo` is driven solely by `hasFixedVersions`; if other remediation types are expected (e.g., configuration changes or workarounds), consider extending this logic to distinguish categories based on advisory content rather than just fixed versions.
## Individual Comments
### Comment 1
<location path="src/main/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyResponseHandler.java" line_range="269-271" />
<code_context>
+ if (title != null) {
+ advisory.title(title);
+ }
+ var identifier = JsonUtils.getTextValue(node, "identifier");
+ if (identifier != null && identifier.startsWith("http")) {
+ advisory.url(URI.create(identifier));
+ }
+ var info = new RemediationInfo().advisory(advisory);
</code_context>
<issue_to_address>
**issue (bug_risk):** Creating a URI from unvalidated external data can throw runtime exceptions.
In `buildRemediationInfo`, `URI.create(identifier)` is called solely after a `startsWith("http")` check. If `identifier` is malformed, `URI.create` will throw `IllegalArgumentException` and can disrupt response handling. Please add more robust handling (e.g., catch `IllegalArgumentException` and skip setting the URL, or use a safer parsing/validation helper) so bad advisory data doesn’t break processing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ory attribution (TC-5019) Fix first-wins deduplication bug in TrustifyResponseHandler.toIssues() that dropped remediation data when multiple purl_statuses existed for the same CVE. When the same source:cveId key is encountered again, mergeIssueData() now accumulates fixedIn versions (deduped), adds RemediationInfo entries, and retains the highest CVSS score. Populate the new AdvisoryInfo field on each RemediationInfo using advisory data from Trustify v3 (document_id, title, identifier URL). Set RemediationCategory to VENDOR_FIX when fixed versions exist. Bump trustify-da-api-model to 2.0.11-SNAPSHOT which includes AdvisoryInfo, updated RemediationInfo with advisory field, and RemediationCategory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…el 2.0.11 RemediationInfo.url() and AdvisoryInfo.url() now accept URI instead of String. Wrap URI.create() calls in try-catch for malformed URLs and guard non-URL advisory identifiers (e.g., GHSA IDs) from being stored as URLs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add advisory RemediationInfo objects to all test fixture issues to match the new buildRemediationInfo fallback behavior in TrustifyResponseHandler. Update remediations summary counts accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
============================================
+ Coverage 56.67% 56.84% +0.16%
- Complexity 836 859 +23
============================================
Files 92 92
Lines 4863 4940 +77
Branches 655 676 +21
============================================
+ Hits 2756 2808 +52
- Misses 1814 1828 +14
- Partials 293 304 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Add vendor_fix and workaround remediation entries to WireMock mock data so the non-empty remediations processing paths in TrustifyResponseHandler (setCvssData lines 237-266, mergeIssueData lines 351-381) are exercised by integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eData Extract three duplicated blocks into private helpers: - extractScoreData: CVSS score/severity extraction from JSON - processVersionRange: version range parsing with dedup flag - processRemediations: remediation array processing with fallback Also add method length convention to CONVENTIONS.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…field The pypi recommendations endpoint deserializes into Vulnerability which only has status/id/justification fields, not remediations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Trustify server now returns a remediations field on the recommendations Vulnerability object. Add @JsonIgnoreProperties to tolerate new fields without breaking deserialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When multiple purl_statuses for the same CVE+source are merged, remediations were blindly appended without checking for duplicates. This produced repeated entries in the output (e.g., the same NO_FIX_PLANNED advisory appearing multiple times). Add isDuplicateRemediation guard using a composite key of (category, details, advisory.id) before adding any remediation item. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…_statuses Reproduces the exact scenario where two purl_statuses within the same vulnerability have identical remediations (WORKAROUND + NO_FIX_PLANNED) with the same advisory document_id, verifying only 2 unique RemediationInfo items appear in the output instead of 4. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and dedup The report.json and batch_report.json fixtures now reflect the actual handler output after advisory attribution and remediation deduplication were added. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes TC-5019 — part of the Trustify v3 migration (TC-4509).
TrustifyResponseHandler.toIssues()used aHashMapkeyed bysource:cveIdwith acontainsKeyguard that discarded subsequentpurl_statusesfor the same CVE, losing their remediations. Now callsmergeIssueData()to accumulate fixedIn versions (deduped), addRemediationInfoentries, and keep the highest CVSS score.document_id,title, andidentifierURL from eachaffectedentry to populateAdvisoryInfoonRemediationInfo. SetsRemediationCategory.VENDOR_FIXwhen fixed versions exist.trustify-da-api-model2.0.9 → 2.0.11-SNAPSHOT (addsAdvisoryInfo, updatedRemediationInfowithadvisoryfield,RemediationCategory)Test plan
testResponseToIssuesMergesRemediationsAcrossAffected— Two affected entries for same CVE merge into one Issue with combined fixedIn and two RemediationInfo itemstestResponseToIssuesWithAdvisoryInfoAttribution— AdvisoryInfo populated correctly (id, title, url for URL identifiers, null url for non-URL identifiers)testResponseToIssuesMergeKeepsHigherCvss— Merge retains higher CVSS score, does not overwrite with lower valueJira: TC-5019
Summary by Sourcery
Merge remediation and advisory data across multiple affected entries for the same issue, ensuring correct CVSS selection and remediation aggregation.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: